Skip to content

Conversation

VincentLanglet
Copy link
Contributor

@VincentLanglet VincentLanglet commented Aug 9, 2025

This could be a solution for the issue phpstan/phpstan#12225

  • Stop merging 0.0 and -0.0 since they have a different toString value
  • Consider === 0.0 like === 0.0 || === -0.0
  • Same for == 0

The other solution I had in mind for the issue was to "just" update ConstantFloat::toString but it has the drawback that

(string) 0.0

would have been considered as '0.0'|'-0.0' while here the constant was really known.

Closes phpstan/phpstan#12225

What could be done to have less changes, would be to modify UnionType::describe to merge 0.0 and -0.0.
But I feel like it's better to be explicit, (since 0.0 and -0.0 have a different behavior) and also, there is already some -0.0 displayed by PHPStan like in the following snippet (and you'll see the already existing inconsistency).
https://phpstan.org/r/0b3cdd4f-b228-4272-a685-621afcf21dd5

cc @staabm (cause I feel like you worked a lot on strict/loose comparison.

@VincentLanglet VincentLanglet marked this pull request as draft August 9, 2025 16:19
@VincentLanglet VincentLanglet changed the title Split 0.0 and -0.0 Better support for -0.0 Aug 9, 2025
@VincentLanglet VincentLanglet force-pushed the poc/12225 branch 4 times, most recently from d183265 to a683afb Compare August 9, 2025 17:16
@VincentLanglet VincentLanglet marked this pull request as ready for review August 9, 2025 17:43
@phpstan-bot
Copy link
Collaborator

This pull request has been marked as ready for review.

@staabm
Copy link
Contributor

staabm commented Aug 10, 2025

in phpstan/phpstan#12225 (comment) you mentioned a always-true check.

will this error be reported after this PR and needs to be test-covered?

@VincentLanglet
Copy link
Contributor Author

in phpstan/phpstan#12225 (comment) you mentioned a always-true check.

will this be reported after this PR and needs to be test-covered?

No, but it's finally another issue because you already has the issue with something like
https://phpstan.org/r/7edaff8c-cf6f-4b62-b6e9-e7a25e4b7300

@ondrejmirtes
Copy link
Member

What if instead of all of these changes, we'd just change ConstantFloatType::toString()?

@VincentLanglet
Copy link
Contributor Author

The other solution I had in mind for the issue indeed was to "just" update ConstantFloat::toString but it has the drawback that

(string) 0.0

would have been considered as '0.0'|'-0.0' while here the constant was really known.

While with this implementation I have the behavior

assertType("'0'", (string) 0.0);
assertType("'-0'", (string) -0.0);

Your call.

@ondrejmirtes
Copy link
Member

Yeah, I'm probably fine with that. We could improve that in the future by adding bool $positive into ConstantFloatType constructor (but not now).

@VincentLanglet
Copy link
Contributor Author

Closing in favor of #4278 then

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

A negative zero float value has a different string representation than positive zero but phpstan is not aware of it

4 participants